Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add regression test for WithDocumentationMode #17803

Merged
merged 3 commits into from
Mar 15, 2017

Conversation

TyOverby
Copy link
Contributor

@TyOverby TyOverby commented Mar 13, 2017

closes #15358

@TyOverby
Copy link
Contributor Author

reviews please: @dotnet/roslyn-compiler

Assert.Equal(1, po2.Features.Count);
}


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: extra empty line

@jcouv
Copy link
Member

jcouv commented Mar 13, 2017

When was the problem fixed?

@TyOverby
Copy link
Contributor Author

@jcouv My best guess would be ac6e765

var po = new CSharpParseOptions().WithFeatures(new[] { new KeyValuePair<string, string>("IOperation", "true") });
Assert.Equal(1, po.Features.Count);
var po2 = po.WithDocumentationMode(DocumentationMode.Diagnose);
Assert.Equal(1, po2.Features.Count);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make sure the values match while we're here and adding a test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

@jaredpar jaredpar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor gripe

Assert.Equal(po2.Features.AsSingleton(), kvp);
}


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Double blank line

Copy link
Member

@jaredpar jaredpar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor gripe

var kvp = new KeyValuePair<string, string>("IOperation", "true");
var po = new CSharpParseOptions().WithFeatures(new[] { kvp });
Assert.Equal(po.Features.AsSingleton(), kvp);
var po2 = po.WithDocumentationMode(DocumentationMode.Diagnose);
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var po2 = po.WithDocumentationMode(DocumentationMode.Diagnose); [](start = 12, length = 63)

I think it would be good to assert DocumentationMode before and after. What if the WithDocumentationMode method does nothing? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tested in the next test in the file.

@AlekseyTs
Copy link
Contributor

LGTM

@TyOverby TyOverby merged commit b5dbed0 into dotnet:master Mar 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Features collection in CSharpParseOptions is reset when changing DocumentationMode
5 participants